Replace eigendecomposition in rotation_vector with direct Rodrigues extraction (+ fix near-identity NaN)#81
Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
V-Francois
left a comment
There was a problem hiding this comment.
I mostly have small comments about your comments (generated by Claude I assume).
The part about vector allocation looks good.
For the matrix to vector function, I don't know these methods. But did you run some test to check that it indeed gives the same results as eigen? Especially near the boundaries?
|
About the closed-form operation, it is basically using the fact that rotation matrices are orthogonal. I did run test before committing. I can do a test.jl and add it to the PR if you want |
Summary
In order to improve performance I propose three fixes :
rotation_vector: use of Rodrigues formula to leverage knowledge of orthogonality for our matrices instead of blindly useeigen, which runs an iterative eigensolver and allocates, when the closed-form Rodrigues extraction needs only a trace and a few subtractions..make_step!but keep the one inwrite_phi_framewhich is enough.To prevent ill defined operation while using the closed-form Rodrigues extraction which divide by
sin θ:sin θ > √epsgeneral case, safe to divide.sin θ ≈ 0, cos θ > 0θ ≈ 0 (near identity), return the zero vector.sin θ ≈ 0, cos θ ≤ 0θ ≈ π, recover the axis from the symmetric part.EDIT : One can run this code to verify the validity of the closed form, especially at the boundaries.